enforce upper limit on MastMissions query radius #3447
Conversation
bmorris3
force-pushed
the
mastmissions-radius-limit
branch
2 times, most recently
from
0204c54 to
941a111
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3447 +/- ##
==========================================
+ Coverage 71.31% 71.48% +0.16%
==========================================
Files 234 234
Lines 20034 20096 +62
==========================================
+ Hits 14287 14365 +78
+ Misses 5747 5731 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
snbianco
left a comment
snbianco
left a comment
There was a problem hiding this comment.
Thank you so much for this PR! There are a few very minor things I commented on, but overall, this looks great.
CHANGES.rst
Outdated
| - Filtering by file extension or by a string column is now case-insensitive in ``MastMissions.filter_products`` | ||
| and ``Observations.filter_products``. [#3427] | ||
|
|
||
| - Raise informative error if MastMissions query radius is too large. [#3447] |
There was a problem hiding this comment.
Nitpick, but could you put MastMissions in double backticks? Just so users know that we're referring to a class.
astroquery/mast/missions.py
Outdated
| Raises | ||
| ------ | ||
| InvalidQueryError | ||
| If the query radius is larger than the limit (30 arcminute). |
There was a problem hiding this comment.
Should be "30 arcminutes" in parentheses.
astroquery/mast/missions.py
Outdated
| Raises | ||
| ------ | ||
| InvalidQueryError | ||
| If the query radius is larger than the limit (30 arcminute). |
|
Hi @bsipocz! Friendly ping. |
|
I'm travelling quite a bit this autumn so things are slower than usual. This one is in the review queue, I'll get back to it once the other big ones are cleared. |
|
Great, thanks @bsipocz 😄 |
bsipocz
left a comment
bsipocz
left a comment
There was a problem hiding this comment.
This will need a rebase, and I have one minor question, otherwise it's ready to go in.
Thanks!
| # maximum supported query radius | ||
| _max_query_radius = 30 * u.arcmin |
There was a problem hiding this comment.
How will we notice if this changes on the server side?
bsipocz
force-pushed
the
mastmissions-radius-limit
branch
from
b208ae9 to
ff01e92
Compare
|
Well, I already did the rebase locally while testing this, so pushed that back and merging once CI is green. |
3 participants
The
astroquery.mast.MastMissionssupportsquery_regionandquery_criteriawithradius <= 0.5 * u.deg. The current uninformative error message whenradius > 0.5 * u.degcomes fromPOST.This PR adds the radius limitation to the docstrings, and raises an
InvalidQueryErrorifradius > 0.5 * u.deg.